Make null handling explicit in toQueryParams and fix misleading test comment#307
Merged
sergey-tihon merged 3 commits intoMar 10, 2026
Conversation
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] Update unit tests for RuntimeHelpers module based on review feedback
Make null handling explicit in Mar 10, 2026
toQueryParams and fix misleading test comment
Member
|
@copilot (Some files need formatting, run |
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Contributor
Author
Applied Fantomas formatting in e80051f — the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
sergey-tihon
merged commit 9c46b0a
into
repo-assist/improve-runtime-helpers-tests-2026-03-4685f59d81f67329
12 checks passed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR clarifies null/Option.None behavior for query parameter serialization by making null handling explicit in RuntimeHelpers.toQueryParams, and fixes a misleading test comment about how F# options are represented at the CLR level.
Changes:
- Add an explicit
nullguard at the start oftoQueryParamsto return an empty query-param list. - Update the test comment to reflect that
Nonefor any F#option<_>is represented asnullat runtime. - Apply Fantomas formatting to the touched areas.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/SwaggerProvider.Runtime/RuntimeHelpers.fs |
Makes toQueryParams return [] for null inputs (avoids NullReferenceException on obj.ToString()). |
tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs |
Corrects test commentary about Option.None CLR representation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| | :? array<Option<string>> as xs -> xs |> toStrArrayOpt name | ||
| | :? array<Option<DateTime>> as xs -> xs |> toStrArrayDateTimeOpt name | ||
| | :? array<Option<DateTimeOffset>> as xs -> xs |> toStrArrayDateTimeOffsetOpt name | ||
| | :? array<Option<Guid>> as xs -> xs |> toStrArray name |
There was a problem hiding this comment.
array<Option<Guid>> is currently serialized via toStrArray, which will call ToString() on Some g (producing e.g. Some ...) and will throw NullReferenceException for None elements (since None is represented as null). This should use the same option-array handling as the other option cases (e.g., toStrArrayOpt) so None values are skipped and Some values serialize as the underlying Guid.
Suggested change
| | :? array<Option<Guid>> as xs -> xs |> toStrArray name | |
| | :? array<Option<Guid>> as xs -> xs |> toStrArrayOpt name |
sergey-tihon
added a commit
that referenced
this pull request
Mar 10, 2026
* Add unit tests for RuntimeHelpers module RuntimeHelpers.fs handles all parameter serialization and HTTP request building for generated API clients, but previously had zero dedicated unit tests. This PR adds 44 focused tests covering: - toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling - toQueryParams: all supported array/option/scalar types, DateTime ISO 8601 serialization, None filtering in optional arrays - combineUrl: trailing/leading slash normalisation - createHttpRequest: HTTP method, path, query params, null filtering - fillHeaders: header addition, null-value skip, Content-Type silence - toStringContent/toTextContent/toStreamContent: content-type headers, error on non-stream input Closes no specific issue; improves baseline test coverage for the runtime serialization layer (Task 9 - Testing Improvements). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304) * Initial plan * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Merge master and fix CI: add RuntimeHelpers unit tests (#305) * Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Initial plan * Fix Build and Test workflow to run on all pull requests, not just PRs targeting master Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Remove unreachable null check from `toQueryParams` fallthrough case (#306) * Initial plan * Fix toQueryParams: remove unreachable null check from _ case Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Make null handling explicit in `toQueryParams` and fix misleading test comment (#307) * Initial plan * Add explicit null guard in toQueryParams and fix test comment Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Apply Fantomas formatting to RuntimeHelpers.fs Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
sergey-tihon
added a commit
that referenced
this pull request
Mar 10, 2026
* Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add concurrency group to CI workflow to cancel redundant builds (#309) When multiple commits are pushed to the same PR branch, GitHub Actions queues a new build for each push. Without a concurrency group, the older builds keep running even though they're now stale. Adding cancel-in-progress: true cancels the in-progress build when a newer commit arrives on the same ref, reducing CI minutes consumed and giving faster feedback on the latest commit. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [Repo Assist] Add unit tests for RuntimeHelpers module (#295) * Add unit tests for RuntimeHelpers module RuntimeHelpers.fs handles all parameter serialization and HTTP request building for generated API clients, but previously had zero dedicated unit tests. This PR adds 44 focused tests covering: - toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling - toQueryParams: all supported array/option/scalar types, DateTime ISO 8601 serialization, None filtering in optional arrays - combineUrl: trailing/leading slash normalisation - createHttpRequest: HTTP method, path, query params, null filtering - fillHeaders: header addition, null-value skip, Content-Type silence - toStringContent/toTextContent/toStreamContent: content-type headers, error on non-stream input Closes no specific issue; improves baseline test coverage for the runtime serialization layer (Task 9 - Testing Improvements). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304) * Initial plan * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Merge master and fix CI: add RuntimeHelpers unit tests (#305) * Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Initial plan * Fix Build and Test workflow to run on all pull requests, not just PRs targeting master Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Remove unreachable null check from `toQueryParams` fallthrough case (#306) * Initial plan * Fix toQueryParams: remove unreachable null check from _ case Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Make null handling explicit in `toQueryParams` and fix misleading test comment (#307) * Initial plan * Add explicit null guard in toQueryParams and fix test comment Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Apply Fantomas formatting to RuntimeHelpers.fs Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> * Initial plan --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
sergey-tihon
added a commit
that referenced
this pull request
Mar 10, 2026
…eference (#310) * Fix $0 in path parameter values treated as regex back-reference (#141) Regex.Replace uses $0, $1, $& etc. as back-references in the replacement string. When a path parameter value contained a literal dollar sign (e.g. "$0something"), Regex.Replace would substitute it with the matched text instead of the literal value. Fix: escape $ characters in the replacement string by replacing "$" with "$$" (the .NET replacement-string escape for a literal dollar sign). This applies to both the v2 SwaggerClientProvider and v3 OpenApiClientProvider. Also adds: - EchoPathController in the test server that returns its string path parameter - A regression test in SpecialCasesControllers.Tests.fs that passes "$0something" through a path parameter and asserts the value is echoed back unchanged - global.json rollForward updated from minor/10.0.103 to latestPatch/10.0.100 so the project builds with any 10.0.1xx SDK available in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix $ in path parameters treated as Regex.Replace back-references (#311) * Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add concurrency group to CI workflow to cancel redundant builds (#309) When multiple commits are pushed to the same PR branch, GitHub Actions queues a new build for each push. Without a concurrency group, the older builds keep running even though they're now stale. Adding cancel-in-progress: true cancels the in-progress build when a newer commit arrives on the same ref, reducing CI minutes consumed and giving faster feedback on the latest commit. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [Repo Assist] Add unit tests for RuntimeHelpers module (#295) * Add unit tests for RuntimeHelpers module RuntimeHelpers.fs handles all parameter serialization and HTTP request building for generated API clients, but previously had zero dedicated unit tests. This PR adds 44 focused tests covering: - toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling - toQueryParams: all supported array/option/scalar types, DateTime ISO 8601 serialization, None filtering in optional arrays - combineUrl: trailing/leading slash normalisation - createHttpRequest: HTTP method, path, query params, null filtering - fillHeaders: header addition, null-value skip, Content-Type silence - toStringContent/toTextContent/toStreamContent: content-type headers, error on non-stream input Closes no specific issue; improves baseline test coverage for the runtime serialization layer (Task 9 - Testing Improvements). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304) * Initial plan * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Merge master and fix CI: add RuntimeHelpers unit tests (#305) * Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Initial plan * Fix Build and Test workflow to run on all pull requests, not just PRs targeting master Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Remove unreachable null check from `toQueryParams` fallthrough case (#306) * Initial plan * Fix toQueryParams: remove unreachable null check from _ case Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Make null handling explicit in `toQueryParams` and fix misleading test comment (#307) * Initial plan * Add explicit null guard in toQueryParams and fix test comment Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Apply Fantomas formatting to RuntimeHelpers.fs Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> * Initial plan --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
box Option<int>.Nonebeing null and inconsistency with test expectationsif isNull obj then []) at the start oftoQueryParamsto make null handling unambiguousint option)dotnet fantomas) to fix formatting issues✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.